Skip to content

RST sandbox network connections on sandbox close#1414

Merged
ValentaTomas merged 10 commits into
mainfrom
orch-conn-termination
Nov 4, 2025
Merged

RST sandbox network connections on sandbox close#1414
ValentaTomas merged 10 commits into
mainfrom
orch-conn-termination

Conversation

@ValentaTomas
Copy link
Copy Markdown
Member

@ValentaTomas ValentaTomas commented Oct 28, 2025 •
edited
Loading

TODO:

  • Don't return errors that are expected when closing connections
  • Check if RST correctly propagates to client/error in client

@ValentaTomas ValentaTomas requested a review from sitole October 29, 2025 15:15
@ValentaTomas ValentaTomas assigned sitole and unassigned djeebus Oct 29, 2025
@ValentaTomas ValentaTomas marked this pull request as ready for review October 30, 2025 21:34
@ValentaTomas ValentaTomas requested a review from djeebus October 30, 2025 21:42
@ValentaTomas ValentaTomas changed the title Terminate sandbox network connections on sandbox close RST sandbox network connections on sandbox close Oct 30, 2025
ValentaTomas
ValentaTomas commented Oct 30, 2025
View reviewed changes
Comment thread packages/orchestrator/internal/template/build/layer/layer_executor.go
@ValentaTomas
Copy link
Copy Markdown
Member Author

ValentaTomas commented Oct 30, 2025 •
edited
Loading

🚧 Will also add a test.

djeebus
djeebus reviewed Oct 30, 2025
View reviewed changes
Comment thread packages/shared/pkg/proxy/tracking/connection.go
@ValentaTomas
Copy link
Copy Markdown
Member Author

ValentaTomas commented Oct 31, 2025

Will be checking the failed integration tests.

@ValentaTomas
Copy link
Copy Markdown
Member Author

ValentaTomas commented Oct 31, 2025 •
edited
Loading

Will be checking the failed integration tests.

On the rerun they went through, will still check (https://github.com/e2b-dev/infra/actions/runs/18963434009/job/54155528873?pr=1414).

EDIT: Should not be related to this PR changes.

@ValentaTomas ValentaTomas enabled auto-merge (squash) October 31, 2025 16:45
djeebus
djeebus reviewed Nov 4, 2025
View reviewed changes
Comment thread packages/shared/pkg/proxy/tracking/connection.go
m: m,
}

if m != nil {
Copy link
Copy Markdown
Contributor

@djeebus djeebus Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we want m to ever be nil? seems like we could simplify this a little by always tracking connections

Copy link
Copy Markdown
Member Author

@ValentaTomas ValentaTomas Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we don't track this for the incoming conn at all as we never have a use case with cancelling these.

@ValentaTomas ValentaTomas merged commit 8d24e90 into main Nov 4, 2025
46 of 47 checks passed
@ValentaTomas ValentaTomas deleted the orch-conn-termination branch November 4, 2025 00:27
ValentaTomas added a commit that referenced this pull request May 4, 2026
@ValentaTomas
RST sandbox network connections on sandbox close (#1414)
fffe7a7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@djeebus djeebus djeebus approved these changes

@sitole sitole Awaiting requested review from sitole

@jakubno jakubno Awaiting requested review from jakubno jakubno is a code owner

@dobrac dobrac Awaiting requested review from dobrac dobrac is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ValentaTomas @djeebus @dobrac @sitole